Cut the caret comments back to the one a reader would get wrong - #502
Conversation
Only the nested case is not visible from the code: escaping a space when the separator is "|" looks wrong until you know the sub-packet sits inside a field of the packet above it. The rest said what the names and the tests already say.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change removes outdated comments and shortens one serializer comment. No production logic, public declarations, test logic, or assertions changed. ChangesComment cleanup
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This PR removes redundant comments and keeps one clarification without changing runtime behavior, public contracts, dependencies, or tests; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Follow-up to #499, which landed with far more comment than it earned.
Twelve blocks become one line. What goes:
StringSerializer'sescapeSeparatorparameter — the parameter name says what it is, and why the old one was wrong belongs in The separator a string field is escaped against is the one it is joined with #499's commit message, where it already isEscapeSpaces— it was the only one inPacketIndexAttribute;IsOptional,SpecialSeparator,RemoveHeaderandRemoveHashall carry noneWhat stays, one line: escaping a space when the separator is
"|"reads as a mistake until you know the sub-packet sits inside a field of the packet above it.122 tests still pass.
Summary by CodeRabbit